fix(frontend): theme the surfaces that stayed light in dark mode - #10295
Conversation
Covers the seven follow-ups recorded on handover from the dark-theme series: theme preference, non-production dark default, GraphiQL and Mermaid theme binding, token migration for legacy pages, data viewer tone, and schema visualizer dark support. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Resolves the two load-bearing design questions: the non-production signal is PEP 440 pre-release status on the running version, published as a resolved value on the unauthenticated config payload; and the first paint is owned by an inline classification script reading a localStorage mirror rather than by React. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Dual-lens review found three blocking items, all applied: - the planned file layout would have forced a shared/ -> entities/ import, which the frontend layer rules prohibit; the context now lives in shared/ and is filled from the entity, mirroring the existing DatePreferencesProvider - no contrast requirement existed for a feature entirely about color - the pre-paint script had no failure handling despite blocking render Left open for the requester: nothing defines when dark exits alpha, or where the defects the dogfooding surfaces are collected. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
59 tasks across 10 phases, grouped by user story so each ships independently. Records the two Ask First approvals gating phase 3 and the sequencing constraints that would otherwise be discovered late: removing the custom-variant last, memoising the mermaid plugin array, and baselining the light theme before any token swap. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
All seven handover items traced to requirements; verdict minor drift. Corrected one fidelity loss: the dark option's label had been generalised to 'pre-release' when the handover named 'alpha' specifically. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Production now defaults to following the user's browser appearance rather than to light, so the deployment default is three-valued. That removed a limitation instead of adding one: with system as both the cold-start fallback and the production default, a first-ever visit is correct from the first frame rather than flashing. Also: cross-tab sync and semantic-colour palettes move out of scope, system-appearance reaction stays in, the e2e suites stay in, and the branch stacks on bab-dark-theme-app with the PR targeting it. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Withdraws the intermediate revision that defaulted production to the system appearance. It would have put dark-OS production users into the alpha palette without choosing it, which is what the alpha label exists to prevent. The spec now states one governing rule: dark is never reached by inference. That decides both defaults and the pre-paint fallback. Non-production still forces dark ignoring the OS, so an engineer on a light machine still dogfoods it. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Grilling the product core replaced the version-derived deployment default with a plain experimental flag, following the convention both existing experimental settings already use: off by default, enabled in development/docker-compose.yml. Rejected the PEP 440 derivation because pre-release is a property of a version, not a deployment, so it also caught customer betas. Following the existing convention targets the intended deployments and removes a resolver module, the version parsing, a config field, and a dependency. Also: flag off hides the theme field entirely (a light-only picker would leak dark through match-system), stored preferences survive a flag flip, and the organisation-wide default is deferred to GA. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…ct path T031 decided rather than left open: the flag goes in development/docker-compose.yml only, not the root compose file, so a deployment brought up from the root cannot enable dark via the host env var while it is still alpha. Dogfooding defects are reported over Slack, which closes the last open question and makes 'no new defects were found' checkable rather than an absence of evidence. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Stage-2 theme resolution as a pure function: a stored choice plus the browser's appearance resolves to a concrete palette. The browser is consulted only for an explicit SYSTEM choice, so an absent, unreadable or unrecognised choice falls back to light rather than inferring dark from a setting the user never pointed at this application. The React context const was planned for this phase but moved to land with its provider: knip fails on any export without a consumer, so a context with neither a producer nor a reader cannot ship green alone. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The GraphQL sandbox was pinned with forcedTheme="light" and rendered as a bright panel inside the dark shell; Mermaid was pinned to its default palette. Both now follow the active theme and react to a change without a reload. Adds a useResolvedTheme hook that reads the document class rather than a preference. That class is what the CSS already keys off, so a consumer cannot disagree with what the page has painted, and whatever ends up deciding the theme only has to set the class. Adds --content/--content-muted for raised content surfaces and migrates 18 files off hardcoded per-theme variants. Each mapping is byte-identical in both themes: the token resolves to rgb(255,255,255) in light, exactly the bg-white it replaced, and stone-900 in dark. Passes GraphiQL the resolved palette rather than "system" so the sandbox cannot run its own prefers-color-scheme check and drift from the app. Memoises the Mermaid plugin array on the theme, since a fresh array each render would re-run the rehype pipeline continuously. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The remaining dark-mode debt was not the files carrying hardcoded dark: variants -- those render correctly, they are only unmaintainable. It was the files carrying none: 38 unconditional bg-white and 21 bg-gray-* literals that paint bright regardless of theme. The artifacts tab was the clearest case, rendering every row as a white bar on black. Counting dark: occurrences would never have found these, since the defect is the absence of the variant rather than its presence. Adds --content-strong to the content family and migrates all three levels. Every mapping is byte-identical in light -- the token resolves to rgb(255,255,255) where it replaced bg-white -- so light cannot shift. Drops dark: overrides that now duplicate the token's own dark value. Warms the data viewer from neutral to stone, matching the theme's warm greys instead of a colder family. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The criterion counted dark: occurrences, which flagged files that work and missed every file that is actually broken. Restated as the property that matters: no component paints a fixed light surface regardless of theme. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The previous attempt set mermaidConfig.theme and did nothing. Verified in the browser: with the app dark, diagrams still rendered mermaid's default palette (nodeFill #ECECFF, #333 labels). Cause is in mermaid-isomorphic. Its node build forwards mermaidConfig into the page and calls mermaid.initialize; its browser build, which is what the bundler resolves, reads only containerStyle and prefix and calls mermaid.render with mermaid's global defaults. mermaidConfig has therefore never had any effect here -- the pre-existing securityLevel setting was equally inert, though mermaid already defaults to strict so nothing was exposed. mermaid is a phantom dependency, so calling initialize directly would mean promoting it to a direct dependency. Mermaid honours a per-diagram init directive without any global configuration, so the theme travels through the diagram source instead: no new dependency, and a diagram that configures itself keeps its own choice. Only the copy handed to the renderer is rewritten, never the stored document. Verified in the browser: dark gives nodeFill #1f2020 with #cccccc labels, toggling to light returns #ECECFF with #333, without a reload. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The forbidden list banned hex literals but said nothing about bg-white or bg-gray-*, which is the class of defect that left surfaces bright in dark mode. Records why searching for dark: does not find them: the defect is the absence of a variant, not the presence of a wrong one. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
There was a problem hiding this comment.
4 issues found across 67 files
Confidence score: 4/5
dev/specs/infp-46-dark-theme-completion/alignment-check.mdreports five decisions while its Decision/Effect table lists seven, which weakens the reliability of the alignment record; reconcile the count and table.dev/specs/infp-46-dark-theme-completion/data-model.mdconflicts withcontracts/graphql-preferences.mdoverInfrahubSetPreferencestheme support and GLOBAL-scope writes, which could mislead implementation; align the shared-scope data model with the contract.dev/specs/infp-46-dark-theme-completion/critiques/critique-20260817-112103.mdreferences FR-022 and FR-021 requirements that no longer resolve, making the P1 finding and remediation difficult to verify; update the references to the current requirements.dev/specs/infp-46-dark-theme-completion/research.mdcites incorrecttheme.csscustom-property and.darkblock details, reducing confidence in the research basis; refresh the evidence against the committed file.
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="dev/specs/infp-46-dark-theme-completion/alignment-check.md">
<violation number="1" location="dev/specs/infp-46-dark-theme-completion/alignment-check.md:7">
P2: Custom agent: **Flag AI Slop and Fabricated Changes**
The Revision 2 section says the grilling pass "produced five decisions," but the Decision/Effect table directly below lists 7 decisions (gate flag, flag-off light-only, dev-config defaults, no removal date, no org-wide default, dev-compose-only, Slack reporting). The edge-case revision likewise says the requester "directed six changes," yet its Direction/Effect table lists 7 rows. Correct both counts to match the tables (or trim the tables) so the narrative counts agree with the content in this fidelity-focused alignment check.</violation>
</file>
<file name="dev/specs/infp-46-dark-theme-completion/data-model.md">
<violation number="1" location="dev/specs/infp-46-dark-theme-completion/data-model.md:100">
P3: This claim conflicts with the sibling contract in the same spec set. `contracts/graphql-preferences.md` adds `theme: Theme` to the shared-scope `InfrahubSetPreferences` mutation and documents GLOBAL-scope writes: "Global set to DARK, no user value → theme.value = DARK, theme.source = GLOBAL" and "Caller lacks global-write permission | mutation with scope GLOBAL". Because the `scope` argument is shared across fields, a theme global writer does exist, so "the global layer simply has no writer" and "global.theme is always null in practice" are not accurate as written. Note `spec.md` FR-003 says the theme preference is user-scoped only, so the two documents disagree about whether theme can be written at GLOBAL scope. Align the data model and the contract (and FR-003) on one of these designs; the current text states a user-only outcome but justifies it with a shared-scope mutation that actually permits a global writer for theme.</violation>
</file>
<file name="dev/specs/infp-46-dark-theme-completion/research.md">
<violation number="1" location="dev/specs/infp-46-dark-theme-completion/research.md:251">
P3: The R6 evidence cites `theme.css` as defining 104 custom properties on `:root` with a `.dark` block at line 60, but the committed file defines only 40 `--` properties on `:root` and its `.dark` block is at line 79. Because this same PR adds tokens rather than removing any, 104 was never an accurate count, so the figure is wrong regardless of when it was measured.
More broadly, the Evidence sections anchor claims to hard-coded line numbers / exact pre-change values that the implementing changes in this same PR immediately invalidate: `graphql/index.tsx:24` passes `forcedTheme="light"` (the commit now passes `forcedTheme={theme}` at line 29), `markdown-with-mermaid.tsx:11` pins `mermaidConfig: { theme: "default" }` (now `securityLevel: "strict"`, and the file moved under `shared/components/editor/markdown/`), `mermaid-diagram.tsx` hardcodes `bg-white` (now `bg-background`), and the data-viewer `bg-neutral-800`/`bg-white` classes (lines 29/58/77/89) are already migrated. The doc is merged with its own implementation, so a reader after merge will find these citations contradict the source.
Since C1 is a standing reference for implementers, replace line-anchored citations with behavior descriptions (symbol + what it does, no line numbers), and correct the property-count/line claims in R6.</violation>
</file>
<file name="dev/specs/infp-46-dark-theme-completion/critiques/critique-20260817-112103.md">
<violation number="1" location="dev/specs/infp-46-dark-theme-completion/critiques/critique-20260817-112103.md:189">
P3: The critique's references to the spec no longer resolve. The P1 finding and its remediation claim name an "FR-022 (contrast)" requirement and describe FR-021 as the "semantic colors stay mutually distinguishable" requirement. In the merged spec.md there is no FR-022 — FR-021 is the contrast requirement ("Text and essential interface elements MUST remain legible against their background"), whose own note states it is *not* about distinguishing semantic colors. A reader navigating from the critique to spec.md will look for FR-022 and misread what FR-021 does. Also, the P5 remediation claim that "FR-003 now states the organisation default reuses the existing global-preference write permission" is not reflected in spec.md: FR-003 explicitly says no organisation-wide default is offered and it is "deferred to the moment the flag is removed". Reconcile these stale references with the merged spec (e.g. cite FR-021 and the actual FR-003 text) so the critique's remediation record matches the requirements it references.</violation>
</file>
Shadow auto-approve: would not auto-approve because issues were found.
Re-trigger cubic
|
|
||
| ## Revision 2 — 2026-08-17, after idea grilling | ||
|
|
||
| A structured grilling pass over the product core (the part that had wobbled twice) produced five |
There was a problem hiding this comment.
P2: Custom agent: Flag AI Slop and Fabricated Changes
The Revision 2 section says the grilling pass "produced five decisions," but the Decision/Effect table directly below lists 7 decisions (gate flag, flag-off light-only, dev-config defaults, no removal date, no org-wide default, dev-compose-only, Slack reporting). The edge-case revision likewise says the requester "directed six changes," yet its Direction/Effect table lists 7 rows. Correct both counts to match the tables (or trim the tables) so the narrative counts agree with the content in this fidelity-focused alignment check.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At dev/specs/infp-46-dark-theme-completion/alignment-check.md, line 7:
<comment>The Revision 2 section says the grilling pass "produced five decisions," but the Decision/Effect table directly below lists 7 decisions (gate flag, flag-off light-only, dev-config defaults, no removal date, no org-wide default, dev-compose-only, Slack reporting). The edge-case revision likewise says the requester "directed six changes," yet its Direction/Effect table lists 7 rows. Correct both counts to match the tables (or trim the tables) so the narrative counts agree with the content in this fidelity-focused alignment check.</comment>
<file context>
@@ -0,0 +1,124 @@
+
+## Revision 2 — 2026-08-17, after idea grilling
+
+A structured grilling pass over the product core (the part that had wobbled twice) produced five
+decisions. **The feature got smaller.** Nothing was added.
+
</file context>
| The chain is the existing one, unchanged — but **theme is exposed at the user scope only**, so no | ||
| interface writes the global layer and `global.theme` is always `null` in practice. The chain | ||
| therefore reduces to `user.theme ?? null`. Nothing needs removing from the backend to achieve this: | ||
| the mutation's `scope` argument is shared across fields, so the global layer simply has no writer. |
There was a problem hiding this comment.
P3: This claim conflicts with the sibling contract in the same spec set. contracts/graphql-preferences.md adds theme: Theme to the shared-scope InfrahubSetPreferences mutation and documents GLOBAL-scope writes: "Global set to DARK, no user value → theme.value = DARK, theme.source = GLOBAL" and "Caller lacks global-write permission | mutation with scope GLOBAL". Because the scope argument is shared across fields, a theme global writer does exist, so "the global layer simply has no writer" and "global.theme is always null in practice" are not accurate as written. Note spec.md FR-003 says the theme preference is user-scoped only, so the two documents disagree about whether theme can be written at GLOBAL scope. Align the data model and the contract (and FR-003) on one of these designs; the current text states a user-only outcome but justifies it with a shared-scope mutation that actually permits a global writer for theme.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At dev/specs/infp-46-dark-theme-completion/data-model.md, line 100:
<comment>This claim conflicts with the sibling contract in the same spec set. `contracts/graphql-preferences.md` adds `theme: Theme` to the shared-scope `InfrahubSetPreferences` mutation and documents GLOBAL-scope writes: "Global set to DARK, no user value → theme.value = DARK, theme.source = GLOBAL" and "Caller lacks global-write permission | mutation with scope GLOBAL". Because the `scope` argument is shared across fields, a theme global writer does exist, so "the global layer simply has no writer" and "global.theme is always null in practice" are not accurate as written. Note `spec.md` FR-003 says the theme preference is user-scoped only, so the two documents disagree about whether theme can be written at GLOBAL scope. Align the data model and the contract (and FR-003) on one of these designs; the current text states a user-only outcome but justifies it with a shared-scope mutation that actually permits a global writer for theme.</comment>
<file context>
@@ -0,0 +1,177 @@
+The chain is the existing one, unchanged — but **theme is exposed at the user scope only**, so no
+interface writes the global layer and `global.theme` is always `null` in practice. The chain
+therefore reduces to `user.theme ?? null`. Nothing needs removing from the backend to achieve this:
+the mutation's `scope` argument is shared across fields, so the global layer simply has no writer.
+When an organisation-wide default is added later, the chain already supports it.
+
</file context>
There was a problem hiding this comment.
reconciled on the contract side, see the note above its behavioural table, be0e7ee
|
|
||
| ### Evidence | ||
|
|
||
| `frontend/packages/ui/src/styles/theme.css` defines 104 custom properties on `:root` with a `.dark` |
There was a problem hiding this comment.
P3: The R6 evidence cites theme.css as defining 104 custom properties on :root with a .dark block at line 60, but the committed file defines only 40 -- properties on :root and its .dark block is at line 79. Because this same PR adds tokens rather than removing any, 104 was never an accurate count, so the figure is wrong regardless of when it was measured.
More broadly, the Evidence sections anchor claims to hard-coded line numbers / exact pre-change values that the implementing changes in this same PR immediately invalidate: graphql/index.tsx:24 passes forcedTheme="light" (the commit now passes forcedTheme={theme} at line 29), markdown-with-mermaid.tsx:11 pins mermaidConfig: { theme: "default" } (now securityLevel: "strict", and the file moved under shared/components/editor/markdown/), mermaid-diagram.tsx hardcodes bg-white (now bg-background), and the data-viewer bg-neutral-800/bg-white classes (lines 29/58/77/89) are already migrated. The doc is merged with its own implementation, so a reader after merge will find these citations contradict the source.
Since C1 is a standing reference for implementers, replace line-anchored citations with behavior descriptions (symbol + what it does, no line numbers), and correct the property-count/line claims in R6.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At dev/specs/infp-46-dark-theme-completion/research.md, line 251:
<comment>The R6 evidence cites `theme.css` as defining 104 custom properties on `:root` with a `.dark` block at line 60, but the committed file defines only 40 `--` properties on `:root` and its `.dark` block is at line 79. Because this same PR adds tokens rather than removing any, 104 was never an accurate count, so the figure is wrong regardless of when it was measured.
More broadly, the Evidence sections anchor claims to hard-coded line numbers / exact pre-change values that the implementing changes in this same PR immediately invalidate: `graphql/index.tsx:24` passes `forcedTheme="light"` (the commit now passes `forcedTheme={theme}` at line 29), `markdown-with-mermaid.tsx:11` pins `mermaidConfig: { theme: "default" }` (now `securityLevel: "strict"`, and the file moved under `shared/components/editor/markdown/`), `mermaid-diagram.tsx` hardcodes `bg-white` (now `bg-background`), and the data-viewer `bg-neutral-800`/`bg-white` classes (lines 29/58/77/89) are already migrated. The doc is merged with its own implementation, so a reader after merge will find these citations contradict the source.
Since C1 is a standing reference for implementers, replace line-anchored citations with behavior descriptions (symbol + what it does, no line numbers), and correct the property-count/line claims in R6.</comment>
<file context>
@@ -0,0 +1,323 @@
+
+### Evidence
+
+`frontend/packages/ui/src/styles/theme.css` defines 104 custom properties on `:root` with a `.dark`
+block at line 60. The light palette is **warm** — `--background: var(--color-stone-100)`,
+`--foreground: var(--color-stone-800)`, `--card`/`--panel` built from `stone`/`gray` stops.
</file context>
There was a problem hiding this comment.
replaced the brittle numbers with a structural description, be0e7ee
|
|
||
| **Applied to `spec.md`:** | ||
|
|
||
| - **P1** — added **FR-022** (contrast) and **SC-009** (verifiable contrast outcome). |
There was a problem hiding this comment.
P3: The critique's references to the spec no longer resolve. The P1 finding and its remediation claim name an "FR-022 (contrast)" requirement and describe FR-021 as the "semantic colors stay mutually distinguishable" requirement. In the merged spec.md there is no FR-022 — FR-021 is the contrast requirement ("Text and essential interface elements MUST remain legible against their background"), whose own note states it is not about distinguishing semantic colors. A reader navigating from the critique to spec.md will look for FR-022 and misread what FR-021 does. Also, the P5 remediation claim that "FR-003 now states the organisation default reuses the existing global-preference write permission" is not reflected in spec.md: FR-003 explicitly says no organisation-wide default is offered and it is "deferred to the moment the flag is removed". Reconcile these stale references with the merged spec (e.g. cite FR-021 and the actual FR-003 text) so the critique's remediation record matches the requirements it references.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At dev/specs/infp-46-dark-theme-completion/critiques/critique-20260817-112103.md, line 189:
<comment>The critique's references to the spec no longer resolve. The P1 finding and its remediation claim name an "FR-022 (contrast)" requirement and describe FR-021 as the "semantic colors stay mutually distinguishable" requirement. In the merged spec.md there is no FR-022 — FR-021 is the contrast requirement ("Text and essential interface elements MUST remain legible against their background"), whose own note states it is *not* about distinguishing semantic colors. A reader navigating from the critique to spec.md will look for FR-022 and misread what FR-021 does. Also, the P5 remediation claim that "FR-003 now states the organisation default reuses the existing global-preference write permission" is not reflected in spec.md: FR-003 explicitly says no organisation-wide default is offered and it is "deferred to the moment the flag is removed". Reconcile these stale references with the merged spec (e.g. cite FR-021 and the actual FR-003 text) so the critique's remediation record matches the requirements it references.</comment>
<file context>
@@ -0,0 +1,216 @@
+
+**Applied to `spec.md`:**
+
+- **P1** — added **FR-022** (contrast) and **SC-009** (verifiable contrast outcome).
+- **P5** — FR-003 now states the organisation default reuses the existing global-preference write
+ permission.
</file context>
There was a problem hiding this comment.
added a post-merge note with the renumbering, the critique is a point in time record so I did not rewrite its findings, be0e7ee
Scrollbars, native form controls and their popups, autofill highlights and the canvas behind overscroll are painted by the browser, not by CSS, and stay light in dark mode unless color-scheme says otherwise. Declare it alongside each palette so the two can never drift apart. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The token already resolves to this value in dark, so the override only duplicated it. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
There was a problem hiding this comment.
1 issue found across 67 files
Confidence score: 3/5
- In
frontend/app/src/pages/graphql/index.tsx, introducingforcedTheme={theme}fromuseResolvedTheme()changes GraphQL sandbox behavior, but the linked task indev/specs/infp-46-dark-theme-completion/tasks.mdindicates that change lacks corresponding test evidence; this creates a concrete regression risk for sandbox theming behavior—add or reference targeted tests for light/dark resolution before merging.
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="dev/specs/infp-46-dark-theme-completion/tasks.md">
<violation number="1" location="dev/specs/infp-46-dark-theme-completion/tasks.md:127">
P2: Custom agent: **Flag AI Slop and Fabricated Changes**
T036 themes the GraphQL sandbox (`forcedTheme={theme}` from `useResolvedTheme()` in `frontend/app/src/pages/graphql/index.tsx`) but that changed behavior has no test. T037, the task that would test it, is unchecked, and the PR reports only 8 Mermaid tests. This doc itself warns the relied-on GraphiQL behavior is undocumented public API ('a test is what protects the binding across upgrades'), so the untested change can regress silently. Add the regression test (T037) asserting the sandbox receives the resolved value and follows a theme change before merging.</violation>
</file>
Shadow auto-approve: would not auto-approve because issues were found.
Re-trigger cubic
|
|
||
| ## Phase 5: User Story 3 — GraphQL sandbox (P2) | ||
|
|
||
| - [x] T036 [US3] Replace `forcedTheme="light"` in `frontend/app/src/pages/graphql/index.tsx` with the resolved theme from the shared context. ⚠ Pass `"light"`/`"dark"` only — never `"system"`, or GraphiQL runs its own `prefers-color-scheme` detection and can disagree with the application. |
There was a problem hiding this comment.
P2: Custom agent: Flag AI Slop and Fabricated Changes
T036 themes the GraphQL sandbox (forcedTheme={theme} from useResolvedTheme() in frontend/app/src/pages/graphql/index.tsx) but that changed behavior has no test. T037, the task that would test it, is unchecked, and the PR reports only 8 Mermaid tests. This doc itself warns the relied-on GraphiQL behavior is undocumented public API ('a test is what protects the binding across upgrades'), so the untested change can regress silently. Add the regression test (T037) asserting the sandbox receives the resolved value and follows a theme change before merging.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At dev/specs/infp-46-dark-theme-completion/tasks.md, line 127:
<comment>T036 themes the GraphQL sandbox (`forcedTheme={theme}` from `useResolvedTheme()` in `frontend/app/src/pages/graphql/index.tsx`) but that changed behavior has no test. T037, the task that would test it, is unchecked, and the PR reports only 8 Mermaid tests. This doc itself warns the relied-on GraphiQL behavior is undocumented public API ('a test is what protects the binding across upgrades'), so the untested change can regress silently. Add the regression test (T037) asserting the sandbox receives the resolved value and follows a theme change before merging.</comment>
<file context>
@@ -0,0 +1,213 @@
+
+## Phase 5: User Story 3 — GraphQL sandbox (P2)
+
+- [x] T036 [US3] Replace `forcedTheme="light"` in `frontend/app/src/pages/graphql/index.tsx` with the resolved theme from the shared context. ⚠ Pass `"light"`/`"dark"` only — never `"system"`, or GraphiQL runs its own `prefers-color-scheme` detection and can disagree with the application.
+- [ ] T037 [US3] Test that the sandbox receives the resolved value and follows a theme change. ⚠ The relied-upon behaviour (reactive `forcedTheme`, and picker-hiding when set) is not documented public API — it was verified against `graphiql@5.2.4`'s bundled source, so a test is what protects the binding across upgrades.
+
</file context>
There was a problem hiding this comment.
partially addressed, the hook is now tested directly in 207326e, the graphiql binding itself still wants an e2e assertion, noted on T037
The open navigation item and the active sort/filter icons painted a fixed indigo in both themes. Measured against the surface each actually sits on, it cleared neither: 3.7:1 in light and 4.3:1 in dark, against the 4.5:1 AA needs for 16px text. An --active token takes the dark end of the ramp in light and the light end in dark, reaching 6.3:1 and 6.2:1. The icon call sites keep their exact light appearance, since indigo-700 is what the light token resolves to; only the sidebar shifts, and only because it was the worse of the two failures. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The seven avatar colours and the template start-button tile all painted a near-white -50 surface, so both glared in dark. The tile now draws on an --active-surface token, whose light value is the shade it already used, so light is untouched there. The avatar ramp keeps its dark variants inline: seven hues chosen by a hash carry no meaning a semantic token could hold, and this array is the ramp's only definition, so there is no duplication for a token to prevent. Measuring the letters revealed the ramp was failing light too, at 2.8:1 for yellow and 3.1:1 for green. Moving the text one step to -700 clears AA across all seven in both themes: 4.7-7.2 light, 9.0-12.5 dark. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
There was a problem hiding this comment.
All reported issues were addressed
Shadow auto-approve: would not auto-approve because issues were found.
Re-trigger cubic
There was a problem hiding this comment.
1 issue found across 72 files
Confidence score: 5/5
- In
frontend/app/src/entities/navigation/ui/sidebar/sidebar-menu-item-avatar.tsx, changing all seven light-theme avatar text shades from-600to-700may subtly alter contrast and contradict the stated unchanged light-theme behavior; verify the intended visual result or retain the existing-600shades.
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="frontend/app/src/entities/navigation/ui/sidebar/sidebar-menu-item-avatar.tsx">
<violation number="1" location="frontend/app/src/entities/navigation/ui/sidebar/sidebar-menu-item-avatar.tsx:7">
P3: The PR description states "Light theme behavior remains unchanged," but this change bumps the avatar text shade from the -600 level to -700 in light mode for all seven ramp entries (e.g. `text-green-600` → `text-green-700`). This is a small visible change to light-mode users, not just a dark-mode addition. Confirm it is intended (e.g. for WCAG contrast) and reflect it in the PR description, or revert the -600→-700 bump if light mode must stay pixel-identical.</violation>
</file>
Shadow auto-approve: would not auto-approve because issues were found.
Re-trigger cubic
| "bg-pink-50 text-pink-600", | ||
| "bg-purple-50 text-purple-600", | ||
| "bg-blue-50 text-blue-600", | ||
| "bg-green-50 text-green-700 dark:bg-green-400/15 dark:text-green-300", |
There was a problem hiding this comment.
P3: The PR description states "Light theme behavior remains unchanged," but this change bumps the avatar text shade from the -600 level to -700 in light mode for all seven ramp entries (e.g. text-green-600 → text-green-700). This is a small visible change to light-mode users, not just a dark-mode addition. Confirm it is intended (e.g. for WCAG contrast) and reflect it in the PR description, or revert the -600→-700 bump if light mode must stay pixel-identical.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At frontend/app/src/entities/navigation/ui/sidebar/sidebar-menu-item-avatar.tsx, line 7:
<comment>The PR description states "Light theme behavior remains unchanged," but this change bumps the avatar text shade from the -600 level to -700 in light mode for all seven ramp entries (e.g. `text-green-600` → `text-green-700`). This is a small visible change to light-mode users, not just a dark-mode addition. Confirm it is intended (e.g. for WCAG contrast) and reflect it in the PR description, or revert the -600→-700 bump if light mode must stay pixel-identical.</comment>
<file context>
@@ -1,19 +1,24 @@
- "bg-pink-50 text-pink-600",
- "bg-purple-50 text-purple-600",
- "bg-blue-50 text-blue-600",
+ "bg-green-50 text-green-700 dark:bg-green-400/15 dark:text-green-300",
+ "bg-yellow-50 text-yellow-700 dark:bg-yellow-400/15 dark:text-yellow-300",
+ "bg-indigo-50 text-indigo-700 dark:bg-indigo-400/15 dark:text-indigo-300",
</file context>
There was a problem hiding this comment.
the description now calls out the two deliberate light changes, this one was an AA fix, four of the seven hues measured 2.8 to 4.2 against their tiles before
Working on the dark theme has meant editing a class onto the document by hand, so nobody used it long enough to find what is still wrong with it. A development stack now starts dark, and the account menu carries a switch out of it, marked alpha. The theme is off unless a deployment opts in through INFRAHUB_EXPERIMENTAL_DARK_THEME, which only development/docker-compose passes through. A deployment brought up from the root compose file has no way to reach it, which is deliberate while the theme is alpha: the gate hides the switch entirely rather than offering a light-only one, since match-system would otherwise walk a dark-desktop user straight into it. Dark is the default when enabled, rather than the operating system's appearance, because the point is that everyone working on a development stack sees the theme being worked on — deriving it from the desktop would give it only to the engineers already running one. A choice made here is remembered per browser and survives the flag being turned off: disabling a feature is an operator's decision about a deployment and must not reach through and delete what users picked. An inline script applies the remembered theme before the first frame, so a reload does not flash light and then snap to dark once config arrives. Documentation screenshots are pinned to light, or a regeneration run on a development stack would quietly turn every published image dark. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
There was a problem hiding this comment.
0 issues found across 2 files (changes from recent commits).
Confidence score: 5/5
- Automated review surfaced no issues in the provided summaries.
- No files require special attention.
Shadow auto-approve: would not auto-approve. Auto-approval blocked by 19 unresolved issues from previous reviews.
Re-trigger cubic
Base's "color cleanup" reworked three surfaces this branch had already tokenized. Took base's version in all three: each is the more thorough fix, and the mechanical `bg-gray-*` → `bg-content-strong` swaps here were only ever the minimum that made them theme-aware. - Token display: base replaces the ad-hoc classes with the shared `inputStyle`, so the box matches surrounding inputs, and adds `size="md"` to the copy button. Keeping this branch's version would have silently dropped both. - Drop target: base tokenizes the border as well (`border-ring bg-highlight`), where this branch had left `border-custom-blue-500` fixed. - Disabled list: base drops the background entirely for `opacity-60`, which is theme-agnostic by construction rather than by token lookup. Verified base's versions resolve before taking them: `--color-ring` and the `--highlight` gradient both exist, `inputStyle` is imported, and `CopyToClipboard` accepts `size`. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
react-datepicker ships a light-only stylesheet — hardcoded #fff/#f0f0f0 chrome, #216ba5 selection, and no custom properties to hook into — so the calendar popup stayed a white island in dark mode. It slipped past the bg-white/bg-gray sweep because the colours live in a vendored stylesheet, not in our class names. An override sheet re-points every colour it paints at the theme tokens, copying the stock selectors so specificity matches without !important. Layout untouched. Both consumers import it beside the stock sheet. The selection pairs --active with --content — each theme's active indigo against that theme's own content surface — so light selection moves from react-datepicker's foreign blue to the app's indigo, white-on-indigo-700 in light and stone-900-on-indigo-400 in dark, the pairing the contrast audit already measured. Verified live with computed colours in both themes: popup, header, day grid, outside/disabled days, time list, and the selected day + time chip all resolve to tokens. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
There was a problem hiding this comment.
1 issue found across 3 files (changes from recent commits).
Confidence score: 4/5
- In
frontend/app/src/shared/components/inputs/datepicker-overrides.css, keyboard navigation can apply both selected-day classes with equal specificity, so the later CSS rule may produce inconsistent styling for the focused selected date; clarify the intended precedence and add a targeted selector or rule order.
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="frontend/app/src/shared/components/inputs/datepicker-overrides.css">
<violation number="1" location="frontend/app/src/shared/components/inputs/datepicker-overrides.css:85">
P2: When the user navigates the calendar with arrow keys, the focused day gets `.react-datepicker__day--keyboard-selected` in addition to `.react-datepicker__day--selected`. Both rules are single-class selectors (equal specificity), and the keyboard-selected block (line 85) comes after the selected block (line 60), so the later one wins: a selected day under keyboard focus renders as `--active-surface` (a faint tint) instead of `--active`, making the chosen date look unselected while navigating. Stock react-datepicker also keeps a strong color here. Pin the selected value on the combined case, e.g. add a rule for `.react-datepicker__day--selected--keyboard-selected` / prefix with `.react-datepicker__day--selected.react-datepicker__day--keyboard-selected` keeping `--active`, or reorder so `--selected` wins.</violation>
</file>
Shadow auto-approve: would not auto-approve because issues were found.
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
| color: var(--content); | ||
| } | ||
|
|
||
| .react-datepicker__day--keyboard-selected, |
There was a problem hiding this comment.
P2: When the user navigates the calendar with arrow keys, the focused day gets .react-datepicker__day--keyboard-selected in addition to .react-datepicker__day--selected. Both rules are single-class selectors (equal specificity), and the keyboard-selected block (line 85) comes after the selected block (line 60), so the later one wins: a selected day under keyboard focus renders as --active-surface (a faint tint) instead of --active, making the chosen date look unselected while navigating. Stock react-datepicker also keeps a strong color here. Pin the selected value on the combined case, e.g. add a rule for .react-datepicker__day--selected--keyboard-selected / prefix with .react-datepicker__day--selected.react-datepicker__day--keyboard-selected keeping --active, or reorder so --selected wins.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At frontend/app/src/shared/components/inputs/datepicker-overrides.css, line 85:
<comment>When the user navigates the calendar with arrow keys, the focused day gets `.react-datepicker__day--keyboard-selected` in addition to `.react-datepicker__day--selected`. Both rules are single-class selectors (equal specificity), and the keyboard-selected block (line 85) comes after the selected block (line 60), so the later one wins: a selected day under keyboard focus renders as `--active-surface` (a faint tint) instead of `--active`, making the chosen date look unselected while navigating. Stock react-datepicker also keeps a strong color here. Pin the selected value on the combined case, e.g. add a rule for `.react-datepicker__day--selected--keyboard-selected` / prefix with `.react-datepicker__day--selected.react-datepicker__day--keyboard-selected` keeping `--active`, or reorder so `--selected` wins.</comment>
<file context>
@@ -0,0 +1,143 @@
+ color: var(--content);
+}
+
+.react-datepicker__day--keyboard-selected,
+.react-datepicker__month-text--keyboard-selected,
+.react-datepicker__quarter-text--keyboard-selected,
</file context>
There was a problem hiding this comment.
these two classes cannot co-occur, so there is no combined case to pin. react-datepicker's isKeyboardSelected returns !isSelectedDate && isSameDayOrWeek(preSelection) && !isDisabled, so a selected day is excluded from keyboard-selected by construction. the month, quarter and year variants do the same.
Same class of miss as the date picker: react-toastify's default "light" theme paints a hardcoded white card, invisible to the bg-white sweep because the colour lives in a vendored stylesheet. Unlike the date picker it themes through its own custom properties, so re-pointing the light theme's two surface variables at our tokens is the whole fix — the tokens flip under `.dark`, and the toast follows with no `theme` prop and no JSX. The close button is the one colour outside its variable system (hardcoded near-black), overridden alongside. Semantic icon colours (success/error/info/warning) are deliberately untouched — semantic palettes are tracked separately. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
There was a problem hiding this comment.
0 issues found across 2 files (changes from recent commits).
Confidence score: 5/5
- Automated review surfaced no issues in the provided summaries.
- No files require special attention.
Shadow auto-approve: would not auto-approve. Auto-approval blocked by 20 unresolved issues from previous reviews.
Re-trigger cubic
…hite The prefix header fades its overflowing fields out at the right edge by painting a white gradient over them — a white smear in dark mode. No token can replace the white: the panel behind is a per-theme gradient, not a flat colour. A CSS mask fades the content itself to transparent instead, so whatever the panel paints shows through exactly, in any theme, with the overlay element gone entirely. Verified in both themes on the live page. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
There was a problem hiding this comment.
0 issues found across 1 file (changes from recent commits).
Confidence score: 5/5
- Automated review surfaced no issues in the provided summaries.
- No files require special attention.
Shadow auto-approve: would not auto-approve. Auto-approval blocked by 20 unresolved issues from previous reviews.
Re-trigger cubic
A pass over every gradient stop and fixed-palette background the earlier sweeps missed, verified live rather than by grep alone: - Diff-summary skeleton pulsed light gray in dark; the shared Skeleton already shimmers with a translucent tint that works in both themes, so it now just uses it. - The disabled list input had a second disabled site the base branch's cleanup did not reach; same fix as the first (opacity, no fixed background). - The IPAM available-prefix separator dots move to the border token, whose light value is the identical gray they hardcoded. - The resolved-thread card dropped its `to-gray-200` — probing the exact shipped classes showed it has been dead in both themes: the Card paints `background-image: var(--card)` directly, which Tailwind's gradient-stop utilities never touch. The "Resolved" checkbox already carries the state, so the class is removed rather than replaced with an equally dead token version. Restoring a working resolved tint is a design decision left for its own change. Reviewed and deliberately left: the colour-picker trigger's palette gradient (an affordance, not a surface), the always-dark tooltip and data-viewer families and everything rendered inside them, the diff expander strips (the diff palette is tracked separately), and the timeframe pill's fixed dark chip (pre-existing design, legible in both). Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
There was a problem hiding this comment.
0 issues found across 4 files (changes from recent commits).
Confidence score: 5/5
- Automated review surfaced no issues in the provided summaries.
- No files require special attention.
Shadow auto-approve: would not auto-approve. Auto-approval blocked by 20 unresolved issues from previous reviews.
Re-trigger cubic
GraphiQL's dark theme hangs off two HSL triplets with hue 219, a blue-navy that clashed with the warm near-black shell around it. Its every surface is derived from those two variables, so re-pointing base and neutral at stone-900 and stone-200 retunes the whole sandbox — the container now computes to rgb(28,25,23), byte-identical to the app's dark content surface. The triplets are duplicated from the theme as bare HSL components because GraphiQL composes them itself via hsl(var(...)) and cannot read oklch tokens. The selector list mirrors GraphiQL's own dark block so its portalled dialogs and tooltips retune with the main container. Light is untouched: the override only matches body.graphiql-dark, and GraphiQL's light base is already the same white as the app's content token. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
There was a problem hiding this comment.
0 issues found across 1 file (changes from recent commits).
Confidence score: 5/5
- Automated review surfaced no issues in the provided summaries.
- No files require special attention.
Shadow auto-approve: would not auto-approve. Auto-approval blocked by 20 unresolved issues from previous reviews.
Re-trigger cubic
* test: assert the current tree row by its new selected class 94cde56 replaced the hardcoded bg-neutral-100 highlight on the current tree row with semantic tokens for dark theme, but the E2E assertions still pinned the old literal class. The row is highlighted correctly; only the assertions were stale. That one change is why four jobs went red: the same assertion appears eight times across both suites, sharded into E2E-testing-playwright plus the foundation, sites_a and sites_b pytest shards. Assert bg-selected alone rather than the full token set, so a later shadow or hover tweak does not break these again. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix: stop leaking react-aria render-prop keys as class names composeAriaClassName invokes its tailwind callback with react-aria's render-props object, but TreeItem and ListBox both named that parameter resolvedClassName and forwarded it into cn(). cn is clsx-compatible, so an object argument emits its truthy keys as class names -- every tree row and list box carried a literal "hasAction hasChildItems level selectionMode selectionBehavior state id defaultClassName". Neither component uses any render prop, so pass the styles as a plain string. composeAriaClassName already merges the consumer's className after them, which is why the leak was cosmetic rather than a styling bug. Predates the dark-theme work; introduced in 33da87f. Correct callers destructure what they need, e.g. ({ isPressed }) in button.tsx. Add a regression test asserting no render-prop key reaches the row's class list, and that a consumer class still survives. Verified red before the fix, green after. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * test: cover the class-name leak for ListBox too, not just TreeItem Both components had the same defect and the same fix, but only TreeItem had a regression test. Add the ListBox equivalent, and move the file to shared/components/aria/ since it now covers className composition for two unrelated components rather than anything tree-specific. ListBox leaked a different set of keys (layout, orientation, state, defaultClassName) because react-aria's render props differ per component, so each list is asserted separately. Both leak tests were verified red against the unfixed components. The four pass-through tests pass in both states, which is the point: the fix removes the leaked keys and changes nothing else. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
There was a problem hiding this comment.
0 issues found across 10 files (changes from recent commits).
Confidence score: 5/5
- Automated review surfaced no issues in the provided summaries.
- No files require special attention.
Shadow auto-approve: would not auto-approve. Auto-approval blocked by 20 unresolved issues from previous reviews.
Re-trigger cubic
The pre-paint script can only replay a theme this browser has resolved once before, so a first-time visitor arrives with no class at all. Applying the resolved theme from a passive effect let that first commit paint in the wrong palette and then snap to the right one. A layout effect runs before the browser paints the commit, which closes the window entirely. The end-to-end theme assertions were sampling the class once, immediately after a visibility wait, and could observe the same window. They retry now — except the two that run at domcontentloaded, which must keep sampling once: retrying there would also accept a class arriving later from React, which is exactly the regression they exist to catch. Two spec corrections found while checking the above: The token-discipline command could never fail. Git's ERE does not implement \b on every platform, and where it does not, the whole pattern matches nothing -- so "expect no output" passed while the check was seeing no files at all. Where \b does work, it reported five standing false positives, because the legitimate dark:bg-white/N overlays match a bare literal just as well as a fixed palette does. Switched to -P with a lookbehind that excludes the variant. T041 claimed a plugin-array stability test that does not exist, and cannot usefully exist: the React Compiler memoises that array whether or not the useMemo is hand-written, so an assertion on it passes identically with the memo deleted. Recorded the property as held by compilation rather than shipping a test that cannot fail. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The alpha tag rode on the switch itself, so it also appeared on the item offering the way back to light. Light is not pre-release, and tagging the exit route reads as a warning against leaving. It now tags what the item switches *to*, which is the step that actually warrants the caution. The menu item also passed no textValue. MenuItem falls back to the children only when they are a plain string, and these are an icon, a label and the tag, so the fallback landed undefined and react-aria lost the typeahead label. Passing the label restores it. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Reverts the temporary flip (313b133, de36a8a) that defaulted the flag on so the ephemeral preview environment would serve dark without a deployment-side env var. The theme is still alpha, so shipping it on by default is not the posture we want once this leaves draft. Regenerated from the setting rather than reverting the diffs, because the same commit also carried a permanent fix -- the end-of-file-fixer exclusion that stops pre-commit fighting the configuration-reference generator -- which must survive. All four artifacts that record the default are back in step: the root compose file, the OpenAPI schema, the configuration reference, and the frontend REST types. development/docker-compose.yml keeps its `true`. That was always the intended long-term default there, and it is what gives an engineer running the dev stack the theme without extra configuration. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
There was a problem hiding this comment.
1 issue found across 11 files (changes from recent commits).
Confidence score: 4/5
- In
docker-compose.yml, forwardingINFRAHUB_EXPERIMENTAL_DARK_THEMEfrom the host can unexpectedly enable the alpha dark theme in deployments using the root compose file; explicitly unset or gate this variable for production deployments.
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="docker-compose.yml">
<violation number="1" location="docker-compose.yml:96">
P2: While dark is alpha, the root docker-compose.yml still passes INFRAHUB_EXPERIMENTAL_DARK_THEME through from the host environment, so any deployment using the root compose file can flip on the experimental dark theme with a host env var. Per the team's stated dev-only policy, the passthrough should live only in development/docker-compose.yml; root compose should carry no passthrough until the feature exits alpha.</violation>
</file>
Shadow auto-approve: would not auto-approve because issues were found.
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
| INFRAHUB_DELETE_BRANCH_AFTER_MERGE: ${INFRAHUB_DELETE_BRANCH_AFTER_MERGE:-false} | ||
| INFRAHUB_DIFF_UPDATE_AFTER_MERGE: ${INFRAHUB_DIFF_UPDATE_AFTER_MERGE:-true} | ||
| INFRAHUB_DOCS_INDEX_PATH: ${INFRAHUB_DOCS_INDEX_PATH:-/opt/infrahub/docs/build/search-index.json} | ||
| INFRAHUB_EXPERIMENTAL_DARK_THEME: ${INFRAHUB_EXPERIMENTAL_DARK_THEME:-false} |
There was a problem hiding this comment.
P2: While dark is alpha, the root docker-compose.yml still passes INFRAHUB_EXPERIMENTAL_DARK_THEME through from the host environment, so any deployment using the root compose file can flip on the experimental dark theme with a host env var. Per the team's stated dev-only policy, the passthrough should live only in development/docker-compose.yml; root compose should carry no passthrough until the feature exits alpha.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At docker-compose.yml, line 96:
<comment>While dark is alpha, the root docker-compose.yml still passes INFRAHUB_EXPERIMENTAL_DARK_THEME through from the host environment, so any deployment using the root compose file can flip on the experimental dark theme with a host env var. Per the team's stated dev-only policy, the passthrough should live only in development/docker-compose.yml; root compose should carry no passthrough until the feature exits alpha.</comment>
<file context>
@@ -93,7 +93,7 @@ x-infrahub-config: &infrahub_config
INFRAHUB_DIFF_UPDATE_AFTER_MERGE: ${INFRAHUB_DIFF_UPDATE_AFTER_MERGE:-true}
INFRAHUB_DOCS_INDEX_PATH: ${INFRAHUB_DOCS_INDEX_PATH:-/opt/infrahub/docs/build/search-index.json}
- INFRAHUB_EXPERIMENTAL_DARK_THEME: ${INFRAHUB_EXPERIMENTAL_DARK_THEME:-true}
+ INFRAHUB_EXPERIMENTAL_DARK_THEME: ${INFRAHUB_EXPERIMENTAL_DARK_THEME:-false}
INFRAHUB_EXPERIMENTAL_GRAPHQL_ENUMS: ${INFRAHUB_EXPERIMENTAL_GRAPHQL_ENUMS:-false}
INFRAHUB_EXPERIMENTAL_VALUE_DB_INDEX: ${INFRAHUB_EXPERIMENTAL_VALUE_DB_INDEX:-false}
</file context>
| INFRAHUB_EXPERIMENTAL_DARK_THEME: ${INFRAHUB_EXPERIMENTAL_DARK_THEME:-false} | |
| INFRAHUB_EXPERIMENTAL_DARK_THEME: |
There was a problem hiding this comment.
the passthrough cannot be withheld from the root file, and the default is now false again (d17a7b1), so this sits exactly where its two siblings do:
INFRAHUB_EXPERIMENTAL_DARK_THEME: ${INFRAHUB_EXPERIMENTAL_DARK_THEME:-false}
INFRAHUB_EXPERIMENTAL_GRAPHQL_ENUMS: ${INFRAHUB_EXPERIMENTAL_GRAPHQL_ENUMS:-false}
INFRAHUB_EXPERIMENTAL_VALUE_DB_INDEX: ${INFRAHUB_EXPERIMENTAL_VALUE_DB_INDEX:-false}
that block is generated from Settings by release.gen-config-env, and validate-docker-compose-env-vars fails on any drift, so deleting the line just turns that job red. keeping it out would mean moving dark_theme into DevelopmentSettings, which /api/config does not publish, so the gating rule could then only read it by widening an unauthenticated endpoint to expose development-only settings.
T031 in tasks.md records this, including the original dev-only intent and why it was reversed. what that intent protected still holds: production is light because the default is false and the frontend gates on the flag.
The visualizer deliberately does no theme detection of its own, so it renders whatever the embedder resolves. Passing the resolved theme is the whole integration. Pins the submodule at the squash-merge commit on main rather than at the branch commit this branch had been carrying. The two are not the same object: the upstream PR was squash-merged, so neither the old pin nor the branch tip is an ancestor of main, and the old pin would break every checkout once the branch is deleted. It was also three commits behind, missing the upstream review fixes. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
There was a problem hiding this comment.
1 issue found across 2 files (changes from recent commits).
Confidence score: 4/5
- In
frontend/packages/schema-visualizer,getVsCodeThemecan fall back tovscode-lighton the first VS Code webview paint when no explicitthemeprop is provided, causing an initial light render; ensure theme detection waits for or correctly handles VS Code’s theme class/attribute.
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="frontend/packages/schema-visualizer">
<violation number="1" location="frontend/packages/schema-visualizer:1">
P2: On first paint in the VSCode-embedded path (no explicit `theme` prop), getVsCodeTheme falls through to `vscode-light` because VS Code has not yet added its theme class/attribute, so the webview renders light and only re-paints once the MutationObserver fires. This causes a light flash for dark-theme users opening the visualizer. Consider initializing the default from VS Code's resolved theme or deferring paint until the theme is known.</violation>
</file>
Shadow auto-approve: would not auto-approve because issues were found.
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
| @@ -1 +1 @@ | |||
| Subproject commit f7d3cc5af409e9db7916947e33b887737a626d4d | |||
| Subproject commit c581b806f9b7e3c640ac0e346a151c99de980e6b | |||
There was a problem hiding this comment.
P2: On first paint in the VSCode-embedded path (no explicit theme prop), getVsCodeTheme falls through to vscode-light because VS Code has not yet added its theme class/attribute, so the webview renders light and only re-paints once the MutationObserver fires. This causes a light flash for dark-theme users opening the visualizer. Consider initializing the default from VS Code's resolved theme or deferring paint until the theme is known.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At frontend/packages/schema-visualizer, line 1:
<comment>On first paint in the VSCode-embedded path (no explicit `theme` prop), getVsCodeTheme falls through to `vscode-light` because VS Code has not yet added its theme class/attribute, so the webview renders light and only re-paints once the MutationObserver fires. This causes a light flash for dark-theme users opening the visualizer. Consider initializing the default from VS Code's resolved theme or deferring paint until the theme is known.</comment>
<file context>
@@ -1 +1 @@
-Subproject commit f7d3cc5af409e9db7916947e33b887737a626d4d
+Subproject commit c581b806f9b7e3c640ac0e346a151c99de980e6b
</file context>
There was a problem hiding this comment.
this path is not reachable from infrahub, and the code is not this PR's to change.
getVsCodeTheme lives in src/webview-entry.tsx, which the package exposes only under the "./webview" export condition. infrahub imports the package root, and index.ts neither exports nor reaches webview-entry. nothing in frontend/app imports schema-visualizer/webview. the one SchemaVisualizer usage we have always passes theme explicitly, so the fallback you describe never runs here.
what moved in this commit is the submodule pointer, from f7d3cc5 to c581b80, which is the squash-merge of the upstream dark-theme PR. so the diff you reviewed is upstream code that already merged in opsmill/infrahub-schema-visualizer. if the VS Code first-paint flash holds up, it wants an issue there rather than a change in this repo.
worth noting the same class of bug did apply to infrahub's own first paint, and is fixed in 96ea50b: a pre-paint script replays the last resolved theme before the first frame, and the provider applies the resolved theme in a layout effect rather than a passive one. an upstream fix would be the analogous shape.
Why
The design system has a complete dark palette, but a number of app surfaces still painted a fixed light background regardless of theme. The GraphQL sandbox rendered as a bright white panel inside the dark shell; the artifacts tab rendered every row as a white bar on black. This clears those, then makes the theme reachable: development stacks now default to dark behind an experimental flag, with a switch in the account menu — so the team dogfoods it while it is alpha.
Non-goals: the per-account theme preference (GraphQL-backed, synced across browsers), the schema visualizer, and semantic colour palettes (status / severity / diff) — each tracked separately under INFP-46.
Warning
This is a stacked PR on #10284 and targets
bab-dark-theme-app, notdevelop. #10284's E2E failures were inherited here rather than caused by this work: they came from stale tree-row class assertions thatdevelophad already fixed, which this branch could not see because its base forked before that fix. Cherry-picked as489d8091e; it will dedupe when #10284 takes the same change. Re-targetdeveloponce #10284 merges.What changed
Behavioural
development/docker-compose.yml) default to the dark theme, gated byINFRAHUB_EXPERIMENTAL_DARK_THEME. The account menu carries a light/dark switch, marked alpha, remembered per browser. With the flag off, the app is light-only and the switch does not exist; a stored choice is retained, never deleted.color-scheme).Implementation notes
@infrahub/ui:useResolvedTheme(reads the document class, so a consumer cannot disagree with what the page painted), the control context, the storage helpers, a ready-madeThemeSwitchMenuItem, and aThemeProviderthat takescanChooseanddefaultThemeand holds no opinion beyond them. The application's provider keeps only the policy — read the flag, default a fresh user to dark — and is six lines. It could not have moved wholesale: it callsuseConfig(), so the design system would have had to know Infrahub's config endpoint and query client, inverting the dependency the package exists to avoid.--content,--content-muted,--content-strongfor raised content surfaces;--active,--active-surfacefor the element the user is acting on.mermaid.initialize({ theme })— the mechanism its renderer documents for browsers, sincemermaid-isomorphic's browser build renders against mermaid's global config and silently discards the rehype plugin'smermaidConfig(the pre-existingsecurityLevel: "strict"was equally inert; it is now actually enforced).mermaidbecomes a declared dependency, resolving to the version already in the tree. The call is shaped as a rehype plugin sequenced before the renderer, because the React Compiler drops render-phase side effects — a visual test caught exactly that. An author's own%%{init}%%directive still wins, by mermaid's own precedence.A note on how the defects were found, since it shaped the diff: the files carrying hardcoded
dark:variants render correctly — they are only unmaintainable. The broken surfaces were the ones carrying no variant at all, 38 unconditionalbg-whiteand 21bg-gray-*. Grepping fordark:finds the former and none of the latter. The styling guideline now records this.What stayed the same, and two deliberate exceptions
bg-contentresolves torgb(255,255,255), byte-identical to thebg-whiteit replaced.experimental_features.dark_themeon the unauthenticated/api/config(OpenAPI schema regenerated accordingly). One newly declared dependency:mermaid^11— it was already in the tree viamermaid-isomorphicand resolves to the same installed version, so no new code ships.Related context
Part of INFP-46. Spec, plan and task breakdown are in
dev/specs/infp-46-dark-theme-completion/and included here.Still to come: the per-account theme preference (GraphQL-backed), and dark support in
opsmill/infrahub-schema-visualizer. The contrast audit is done and recorded indev/specs/infp-46-dark-theme-completion/contrast-audit.md— zero AA failures across twelve routes in both themes.Documentation updates
dev/knowledge/frontend/theming.md— new: how theming works end to end; the recipe for changing a colour in one theme only.docs/docs/faq/faq.mdx— user-facing FAQ entry for the theme; a changelog fragment underchangelog/.dev/guidelines/frontend/styling.md— forbidden list now covers fixed-palette surface classes, with the reasoning above.dev/specs/infp-46-dark-theme-completion/— spec, plan, research, contracts, tasks, critique and alignment check.How to review
Start with
frontend/packages/ui/src/styles/theme.css(the tokens) andfrontend/packages/ui/src/theme/(the primitives), thentheme-provider.tsx(the policy) andmarkdown-with-mermaid.tsx. The other ~50 files are mechanical class-to-token swaps.How to test
Manually:
pnpm dev— the app starts dark; switch themes from the account menu (bottom of the sidebar), then check the GraphQL sandbox, a Mermaid diagram, the artifacts tab and the data viewer. E2E journeys (first paint, persistence, flag-off) are intests/e2e/theme.spec.ts.Verified in a live browser with computed-colour assertions rather than by eye:
rgb(31,32,32)rgb(236,236,255)rgb(204,204,204)rgb(51,51,51)bg-contentstone-900rgb(255,255,255)GraphiQL confirmed both ways, and both surfaces follow a live theme toggle without a reload.
Impact & rollout
/api/config.INFRAHUB_EXPERIMENTAL_DARK_THEME, generated into the rootdocker-compose.ymlfromSettingsalongside its two experimental siblings, and present indevelopment/docker-compose.yml. Defaults tofalse.development/docker-compose.ymlsets ittrue, so the dev stack serves the theme without extra configuration.Screenshots
Captured on a live stack at 1440×900 — same pages, same data, only the theme differing.
Home
Object list
Proposed change with a Mermaid diagram — the diagram itself follows the theme:
GraphQL sandbox — follows the app theme instead of being pinned light:
Login — themed before a session exists:
Checklist
+dark-theme.added.md; theci/skip-changeloglabel is off